Add CBOR Protocol Support for Request Marshalling and Response Unmarshalling #3942
Conversation
There was a problem hiding this comment.
We are missing the generator change that when a service says they are going to use Cbor that the generated nuspec for that service includes a reference to the new AWSSDK.Extensions.CborProtocol extension package.
I modified SQS in my local workspace to use Cbor and confirmed the marshallers were using Cbor but the generated AWSSDK.SQS.nuspec only list AWSSDK.Core as a dependency.
generator/.DevConfigs/6a90e4df-251e-45f8-808a-1a088cf0b25e.json
Outdated
Show resolved
Hide resolved
generator/.DevConfigs/6a90e4df-251e-45f8-808a-1a088cf0b25e.json
Outdated
Show resolved
Hide resolved
| @@ -67,6 +69,10 @@ | |||
| {"<#=member.PropertyName#>", payload => new <#=member.DetermineType()#>Unmarshaller().Unmarshall(context);}, | |||
| <# | |||
| } | |||
| else if (protocol == "Cbor") | |||
| { | |||
| // TODO | |||
There was a problem hiding this comment.
Make the generator fail if we go in here to avoid us accidently shipping a service update and thinking it was successful
There was a problem hiding this comment.
Currently working on event streaming and planning to add the code once a new PR is approved.
| @@ -104,6 +110,10 @@ | |||
| { "<#=member.PropertyName#>", payload => new <#=this.Config.ClassName#>EventStreamException(Encoding.UTF8.GetString(payload.Payload), new <#=member.ModelShape#>Unmarshaller().Unmarshall(EventStreamUtils.ConvertMessageTo<#=protocol#>Context(payload))) }, | |||
| <# | |||
| } | |||
| else if (protocol == "Cbor") | |||
| { | |||
| // TODO | |||
There was a problem hiding this comment.
Make the generator fail if we go in here to avoid us accidently shipping a service update and thinking it was successful
|
@normj This is a huge miss, thank you for pointing it out , will work on updating |
muhammad-othman
force-pushed
the
cbor-protocol
branch
2 times, most recently
from
74051ae to
ed8a864
Compare
muhammad-othman
force-pushed
the
cbor-protocol
branch
from
ed8a864 to
98d1642
Compare
|
@muhammad-othman there is a merge conflict. will wait on that fix before hitting approve |
…hy Version 1.58.0
…and Smithy v1.58.0
…e file updates. (#3914) * Updated notice files for system.formats.cbor. * Updated the build to package AWSSDK.Extensions.CborProtocol.
muhammad-othman
force-pushed
the
cbor-protocol
branch
from
1e25214 to
7ce0b53
Compare
Description
This PR is for
CBORprotocol feature branch todevelopment.It includes the following PRs that were reviewed as part of this feature branch:
Motivation and Context
DOTNET-7384Testing
AWSSDK.Extensions.CborProtocolproject.CloudWatchandSecretsManageroperations with CBOR.Screenshots (if appropriate)
Types of changes
Checklist
License